Skip to content

fix(taglist): for property filter#4084

Open
cnjhb wants to merge 1 commit intoawesomeWM:masterfrom
cnjhb:taglist
Open

fix(taglist): for property filter#4084
cnjhb wants to merge 1 commit intoawesomeWM:masterfrom
cnjhb:taglist

Conversation

@cnjhb
Copy link
Copy Markdown
Contributor

@cnjhb cnjhb commented Mar 13, 2026

table filter interfere with property filter, so we need to remove table filter from the instance.

In practical terms, this means we cannot modify the filter property of a taglist instance.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.48%. Comparing base (f1388f9) to head (42011d1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4084      +/-   ##
==========================================
+ Coverage   90.45%   90.48%   +0.03%     
==========================================
  Files         941      942       +1     
  Lines       60390    60402      +12     
  Branches     1145     1145              
==========================================
+ Hits        54628    54657      +29     
+ Misses       5252     5238      -14     
+ Partials      510      507       -3     
Files with missing lines Coverage Δ
lib/awful/widget/taglist.lua 89.73% <100.00%> (+3.09%) ⬆️
tests/test-taglist.lua 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cnjhb cnjhb changed the title fix(taglist): for filter fix(taglist): for property filter Mar 13, 2026
@Aire-One
Copy link
Copy Markdown
Member

Aire-One commented Mar 15, 2026

The fix seems reasonable at first glance, but we do a lot of these gears.table.crush in our constructor-functions. There should be a lot of other properties wrongly copied this way in many modules. Applying this rawset to remove them afterward would just not be maintainable.

A better solution would be to basically refactor the module pattern we use and clearly separate the module level from instance level stuff. I'm currently experimenting Teal. The type system it brings would help a lot in this scenario. The big migration, in addition to being a massive work would probably be not welcomed considering the current status of the project.

@cnjhb
Copy link
Copy Markdown
Contributor Author

cnjhb commented Mar 15, 2026

The fix seems reasonable at first glance, but we do a lot of these gears.table.crush in our constructor-functions. There should be a lot of other properties wrongly copied this way in many modules. Applying this rawset to remove them afterward would just not be maintainable.

A better solution would be to basically refactor the module pattern we use and clearly separate the module level from instance level stuff. I'm currently experimenting Teal. The type system it brings would help a lot in this scenario. The big migration, in addition to being a massive work would probably be not welcomed considering the current status of the project.

It’s true—it would be much better if the language supported object-oriented programming.
I’ve used Penlight’s class before, but they weren’t ideal either.

@cnjhb
Copy link
Copy Markdown
Contributor Author

cnjhb commented Mar 17, 2026

@Aire-One So how should we fix this?

table filter interfere with property filter, so we need to remove table filter from the instance.
Copy link
Copy Markdown
Member

@actionless actionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! mb additionally some note should be added to the docstring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants